Skip to content

fix: address cubic review findings#6

Merged
vieiralucas merged 1 commit intomainfrom
feat/fix-cubic-findings
Mar 26, 2026
Merged

fix: address cubic review findings#6
vieiralucas merged 1 commit intomainfrom
feat/fix-cubic-findings

Conversation

@vieiralucas
Copy link
Copy Markdown
Member

@vieiralucas vieiralucas commented Mar 26, 2026

Fix 7 Cubic findings (3 P1, 4 P2) from PR #5


Summary by cubic

Fixes 7 Cubic findings (3 P1, 4 P2) by hardening FIBP networking, improving error propagation, and making async enqueues concurrent. Increases reliability under concurrency, surfaces decoding issues, and removes temp-file handling for CA certs.

  • Bug Fixes

    • Guard socket sends with a dedicated lock in FibpConnection to prevent interleaved frames from concurrent threads (P1).
    • Map FibpError via _map_enqueue_error_code in batch flush for precise exceptions instead of a generic EnqueueError string (P1).
    • Log warnings when consume message decoding fails in Client and AsyncClient; skip bad frames and fix the inaccurate consume() docstring (P2).
    • Load CA certs via SSLContext.load_verify_locations(cadata=...) to avoid temp files and related risks (P2).
  • Performance

    • Make AsyncClient.enqueue_many send per-queue requests concurrently with asyncio.gather to improve enqueue throughput.

Written for commit 4166053. Summary will update on new commits.

- log warning instead of silently swallowing decode errors in consume
  iterators (client.py, async_client.py)
- add threading.Lock for socket sends in FibpConnection to prevent
  interleaved frames from concurrent threads (fibp.py)
- fix consume() docstring to drop false reconnect claim (client.py)
- use asyncio.gather() in async enqueue_many for concurrent per-queue
  requests (async_client.py)
- map FibpError via _map_enqueue_error_code in batch flush instead of
  wrapping as raw EnqueueError string (batcher.py)
- use SSLContext.load_verify_locations(cadata=...) to avoid temp file
  for ca cert (fibp.py)
@vieiralucas vieiralucas merged commit 474d01d into main Mar 26, 2026
2 of 3 checks passed
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant